fix(request): linear SSE buffering with pre-append size check - #418

Summary

Addresses deep-audit finding REQ-HIGH-03: convertSseToJson in lib/request/response-handler.ts buffered the entire SSE stream via repeated fullText += decoder.decode(...). That is O(n) per append on V8, producing O(n^2) total work for large streams, and the MAX_SSE_SIZE (10 MB) cap was enforced AFTER the append, so peak memory briefly held chunk + 10 MB before throwing.

Fix

• Accumulate decoded chunks in a string[] and join('') once at the end (linear total work).
• Track a running totalSize counter.
• Check totalSize + decoded.length > MAX_SSE_SIZE BEFORE appending each chunk, so the cap is enforced pre-allocation and peak memory is bounded to the cap rather than cap + chunk.

Tests

New dedicated regression file test/response-handler-sse-buffer.test.ts (3 tests), isolated from the existing 860-line response-handler.test.ts to avoid whole-file reformatting noise in the diff:

• Accumulates many ~128 KB chunks summing just under the 10 MB cap and parses successfully (exercises the multi-chunk accumulation path).
• Asserts the throw fires on the FIRST chunk that would push past the cap (pre-append), and that no further reads happen after the throw.
• Asserts a single oversize first chunk is rejected before it is retained in any buffer.

Verification

• npm test -- response-handler — 56 passed (was 53; +3 regression tests)
• npm run typecheck — clean
• npm run lint — clean

Constraints observed

• No as any, @ts-ignore, or @ts-expect-error.
• Changes are confined to lib/request/response-handler.ts and a new co-located response-handler-scoped test file.
• No amend, no force-push.

note: greptile review for oc-chatgpt-multi-auth. cite files like lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.

Greptile Summary

the pr correctly replaces O(n²) string concatenation with an array-then-join pattern and enforces the 10 MB cap before each append. the follow-up commit (afd1fff) also switched from decoded.length (utf-16 code units) to Buffer.byteLength(decoded, "utf8"), addressing the prior review comment — though value.byteLength would be simpler, avoids re-encoding, and is slightly tighter at multi-byte chunk boundaries.

Confidence Score: 5/5

safe to merge; both remaining findings are P2 style suggestions with no correctness impact

the core fix is correct — linear accumulation, pre-append guard, and proper utf-8 byte counting all work. the Buffer.byteLength vs value.byteLength difference is a minor efficiency/elegance nit, not a bug; totals are equivalent over the full stream. four targeted regression tests cover all three original scenarios plus the utf-8 edge case. no security, data-loss, or concurrency issues.

no files require special attention

Important Files Changed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[reader.read] --> B{done?}
    B -- yes --> G[chunks.join]
    B -- no --> C[decoder.decode value stream:true]
    C --> D[decodedBytes = Buffer.byteLength decoded utf8]
    D --> E{totalSize + decodedBytes > MAX_SSE_SIZE?}
    E -- yes --> F[throw + reader.cancel]
    E -- no --> H[chunks.push decoded\ntotalSize += decodedBytes]
    H --> A
    G --> I[parseSseStream fullText]
    I --> J{finalResponse?}
    J -- no --> K[return plain text Response]
    J -- yes --> L[return JSON Response]

This is a comment left during a code review.
Path: lib/request/response-handler.ts
Line: 792-800

Comment:
**unnecessary re-encode; use `value.byteLength` directly**

`Buffer.byteLength(decoded, "utf8")` re-encodes the already-decoded string back to UTF-8 just to count bytes. `value` is a `Uint8Array` whose `.byteLength` is exactly the raw wire-byte count — no extra allocation or encoding pass needed. additionally, when `TextDecoder` running in streaming mode buffers 1–3 trailing bytes of a split multi-byte sequence, `Buffer.byteLength(decoded, "utf8")` excludes those bytes from the current chunk (they appear in the next `decoded` call instead); `value.byteLength` counts them immediately, making the guard slightly tighter at chunk boundaries.

```suggestion
			const decodedBytes = value.byteLength;
			// Pre-append size check: reject before allocating/retaining the chunk
			// alongside the accumulated buffer. This bounds peak memory to the
			// cap rather than cap + chunk.
			if (totalSize + decodedBytes > MAX_SSE_SIZE) {
				throw new Error(`SSE response exceeds ${MAX_SSE_SIZE} bytes limit`);
			}
			chunks.push(decoded);
			totalSize += decodedBytes;
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: test/response-handler-sse-buffer.test.ts
Line: 28-30

Comment:
**stale comment rationale after switching to `Buffer.byteLength`**

the comment says ascii is chosen because `decoder.decode()` returns a string of the same length as the byte buffer — that was relevant when tracking `decoded.length`. now that the implementation uses `Buffer.byteLength(decoded, "utf8")` (or ideally `value.byteLength`), the byte count is exact for all inputs, so ascii is no longer required for determinism. the 4th test in this file already uses emoji. the comment is now misleading; consider trimming or updating it to match the actual accounting.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (2): Last reviewed commit: "fix(request): count utf-8 bytes in SSE s..." | Re-trigger Greptile}